Skip to content

feat: [Parallel] Do not checkpoint parallel during replay#233

Merged
wangyb-A merged 3 commits intomainfrom
parallel/wait_replay
Mar 19, 2026
Merged

feat: [Parallel] Do not checkpoint parallel during replay#233
wangyb-A merged 3 commits intomainfrom
parallel/wait_replay

Conversation

@wangyb-A
Copy link
Contributor

@wangyb-A wangyb-A commented Mar 19, 2026 •
edited
Loading

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Issue Link, if available

Description

  • Do not checkpoint when replay
  • Add sleep at the end of the example to verify replay
  • Fix failed parallel operation unit tests

Demo/Screenshots

iShot_2026-03-18_17 44 59

Checklist

  • I have filled out every section of the PR template
  • I have thoroughly tested this change

Testing

Unit Tests

Have unit tests been written for these changes? Yes

Integration Tests

Have integration tests been written for these changes? N/A

Examples

Has a new example been added for the change? (if applicable) Yes

@wangyb-A wangyb-A requested a review from a team March 19, 2026 00:55
@zhongkechen
Copy link
Contributor

zhongkechen commented Mar 19, 2026 •
edited
Loading 

    private void handleComplete() {
        if (isOperationCompleted()) {
            return;
        }
        handleSuccess();
        markAlreadyCompleted();
    }

handleSuccess() calls sendOperationUpdate() which will complete this operation already. markAlreadyCompleted(); shouldn't be called here. And there is no lock in this method handleComplete, there is a race condition here handleSuccess could be called multiple times by different child threads.

@zhongkechen
Copy link
Contributor

zhongkechen commented Mar 19, 2026

3 cases in ParallelOperationTest actually failed. The exception from these cases was swallowed by runJoin helper. And also we need to create parallel integration tests in sdk-integration-tests package

@wangyb-A wangyb-A merged commit 84e9bdf into main Mar 19, 2026
11 checks passed
This was referenced Mar 19, 2026
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zhongkechen zhongkechen zhongkechen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wangyb-A @zhongkechen